docs(model): document bedrockGeoRegion and the global-vs-geo tradeoff - #799
Closed
isadeks wants to merge 2 commits into
Closed
docs(model): document bedrockGeoRegion and the global-vs-geo tradeoff#799isadeks wants to merge 2 commits into
isadeks wants to merge 2 commits into
Conversation
The `bedrockGeoRegion` context key added in #764 was documented nowhere — not in the developer guide, not in the canonical model-configuration reference #742 created for exactly this, not in cdk.json. The synth-time error names the key, but only for someone who already knows it exists. Adds a section covering what the key does, its accepted values (derived from what the CDK models: global, us, us-gov, eu, apac, jp, au), and that an unrecognized value fails at synth rather than producing a well-formed ARN for a profile that does not exist. Also states the choice, since it is a real tradeoff rather than a default to accept: `global.` routes to any supported commercial Region for better throughput and resilience under burst, while a geo profile is what a data-residency requirement demands. Plus the two things that fail at runtime rather than synth — a model without a profile in the target geography, and account-level Bedrock access not covering that geography's entitlements. Two existing statements went stale with #764 and are corrected here: the bare-vs-prefixed rule described the grant sites as adding `us.` specifically, and the layer-1 table row described only `bedrockModels`. Both now read in terms of the configured geography. Answers a question that the layering table implies but never states: changing the geography needs a redeploy, because the grants are scoped to profile ARNs resolved at synth, whereas switching among already-granted models does not. Starlight mirror regenerated with `mise //docs:sync`; docs build green.
3 tasks
isadeks
added a commit
that referenced
this pull request
Aug 27, 2026
Addresses all six review findings on the geo switch. The first was a real defect, not a documentation error. FINDING 1 — the one-line rollback was false. `synth -c bedrockGeoRegion=us` granted `us.` profiles while the agent still asked for `global.anthropic.claude-opus-5`, because the stack injected only the AUXILIARY model into the runtime env and never the main one. The main model came from a Python literal that a geography change does not touch, so every task with no per-repo override would fail at turn 0 with AccessDenied. My own PR body half-knew this — the prose said "roll back context and defaults together" while the summary claimed one line. Fixed by removing the divergence rather than re-syncing literals: both models are now injected from the resolved geography, via one shared helper, on BOTH substrates (the ECS task definitions set neither var either, so an ECS task had the same mismatch). Verified by synthesizing both geographies: grants, AgentCore env and ECS env now agree in each, so the rollback genuinely is one context value. Also fixes the fifth stale default the review found, TaskConfig.haiku_model, which I had missed entirely while counting four. FINDING 2 — the generalized allow-list invariant was too weak in both directions. It only proved entries were paired WITHIN the list, which is satisfiable while wrong: three granted models had no `global.` form (a workflow pinning one was rejected at admission), and the reviewer's invented pair passed all 22 tests despite being granted nothing. Replaced with parity against DEFAULT_BEDROCK_MODEL_IDS across the deployable geographies, in both directions, and confirmed the reviewer's exact mutation now fails. FINDING 3 — the `--model` guard let through a well-formed but ungranted model. My justification for that was wrong: I claimed the CLI cannot read `bedrockModels`, but `get-template` recovers the granted set from the profile ARNs using credentials the CLI already needs. Added a `BedrockModelIds` output — a documented contract rather than a regex over CloudFormation — and the guard now rejects an ungranted model, listing what the stack does grant. Also fixes the reverse-compatibility bug: with no geography exported, the bare-id error no longer prescribes `us.`, which a default-global stack does not grant. FINDING 4 — doctor's profile check is relabelled "visible" and its detail states that resolution happens under operator credentials and does not prove the workload role can invoke. The narrow wording was already accurate, but a PASS feeding "All checks passed" still read as readiness. FINDINGS 5-6 — carries #799's documentation so this branch is not self-contradictory, corrects the onboarding and troubleshooting skills that recommended `us.` overrides this branch would now reject (and still called Sonnet 4.6 the default), and fixes the stale "default us" wording in the new output description. One test rewritten rather than re-pinned: the agent's haiku-default test asserted `startswith("us.")` when its stated intent was "must be a profile, not a bare id". It now asserts a geo prefix, so it tests the property instead of a geography that is a deploy-time choice.
Contributor
Author
|
Superseded by #806, which carries this content plus the geo flip and the review fixes. Verified before closing: every distinctive section here ( Closing rather than rebasing, since a rebase would produce an empty diff and #806 needs no doc changes on top. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Part of #740. Closes the documentation gap I raised reviewing #764.
Why
bedrockGeoRegion(added in #764) was documented nowhere — not indocs/guides/DEVELOPER_GUIDE.md, not inModel-configuration.md, not incdk.json. The synth-time error message names the key, but only helps someone who already knows it exists.That matters more than usual because #740 is the documentation tracking issue, and #742 created the canonical model-configuration reference specifically so model settings stop being scattered. A new key that decides which geography every inference profile routes through belongs in it.
What
A section covering:
-c bedrockGeoRegion=globalorcdk.jsoncontext);global,us,us-gov,eu,apac,jp,au;AccessDeniedand nothing to explain why;ANTHROPIC_DEFAULT_HAIKU_MODEL, so a deployment cannot grant one geography while calling another.The tradeoff, stated as a choice rather than a default:
global.routes to any supported commercial Region, giving better throughput and resilience under burst — worth having for tasks that run hours. A geo profile keeps inference in that geography, which is what a data-residency requirement demands. This is the material half for #747, which flips the default toglobal.Plus the two failure modes that surface at runtime rather than synth: a model with no profile in the target geography, and account-level Bedrock access not covering that geography's entitlements.
Two corrections
#764 made two existing statements stale:
us.bedrockModelsonlybedrockGeoRegionOne question the guide implied but never answered
Changing the geography needs a redeploy, because the IAM grants are scoped to explicit profile ARNs resolved at synth. Switching among already-granted models does not — that is a DynamoDB write via layer 4. The layering table implied this; it now says so.
Verification
Starlight mirror regenerated with
mise //docs:sync— both files carry the new content, andmise //docs:buildis green (78 pages). Docs-only; no source changes.